Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleanup after sending to avoid memory leak #334

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

martinfkaeser
Copy link

@martinfkaeser martinfkaeser commented Mar 28, 2024

There is a potential memory leak after sending a request using Server#send() or Client#send():

When a request is sent, the Request object and the associated CompletableFuture are stored in maps (in class Queue and PromiseRepository respectively).
While both are cleaned up when a successful confirmation is received from the communication partner, this is not the case if either an error or no confirmation at all is received:

  • If an error is received only the PromiseRepository but not the Queue is cleaned up.
  • If no message is received there is no way to clean up any of the maps.

This pull request tries to solve these problems:

Queue and PromiseRepository are cleaned up whenever the CompletableFuture resolves, whether successful or not.

This gives users of the send() method the chance to add a timeout to the returned CompletableFuture, e.g. by using:
server.send(sessionIndex, request).orTimeout(10, TimeUnit.SECONDS)
(refer to ServerTest#send_aMessage_promiseCompletesWithTimeout())

As the returned CompletableFuture is identical to one inside send(), after a timeout occurs, the future is completed with a TimeoutException and both maps will be cleaned up.

I'm very interested in your feedback!

By the way, thank you very much for your work on this nice and elegant library!

Best regards,
Martin

- Clean up request and promise maps whenever future completes
- Enables timeout for Client/Server.send()

(cherry picked from commit c801fbb221b4090405e58ed37d414fab06b16ce3)
- Fix and improve tests

(cherry picked from commit 6179a4e2530e432c867a28a32d58bcfe5d639a91)
@martinfkaeser
Copy link
Author

Hi everyone, is there any feedback about the bug and my proposed solution?

Best regards,
Martin

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant